[PATCH 07/24] kill: Support mandating the presence of a userspace signal handler
authorChris Down <chris@chrisdown.name>
Wed, 26 Oct 2022 14:47:36 +0000 (15:47 +0100)
committerChris Hofstaedtler <zeha@debian.org>
Fri, 25 Nov 2022 15:19:08 +0000 (15:19 +0000)
commite64d3775d3f57ccf14584be8e0e9c7c2efae7024
tree78a227b19b4a9f30ad708d016c6935ca488d0fb2
parent5c2120093383ec0199e62c274d1b78ebe03290dc
[PATCH 07/24] kill: Support mandating the presence of a userspace signal handler

In production we've had several incidents over the years where a process
has a signal handler registered for SIGHUP or one of the SIGUSR signals
which can be used to signal a request to reload configs, rotate log
files, and the like. While this may seem harmless enough, what we've
seen happen repeatedly is something like the following:

1. A process is using SIGHUP/SIGUSR[12] to request some
   application-handled state change -- reloading configs, rotating a log
   file, etc;
2. This kind of request is deprecated and removed, so the signal handler
   is removed. However, a site where the signal might be sent from is
   missed (often logrotate or a service manager);
3. Because the default disposition of these signals is terminal, sooner
   or later these applications are going to be sent SIGHUP or similar
   and end up unexpectedly killed.

I know for a fact that we're not the only organistion experiencing this:
in general, signal use is pretty tricky to reason about and safely
remove because of the fairly aggressive SIG_DFL behaviour for some
common signals, especially for SIGHUP which has a particularly ambiguous
meaning. Especially in a large, highly interconnected codebase,
reasoning about signal interactions between system configuration and
applications can be highly complex, and it's inevitable that on occasion
a callsite will be missed.

In some cases the right call to avoid this will be to migrate services
towards other forms of IPC for this purpose, but inevitably there will
be some services which must continue using signals, so we need a safe
way to support them.

This patch adds support for the -r/--require-handler flag, which checks
if a userspace handler is present for the signal being sent. If it is
not, the process will be skipped.

With this flag we can enforce that all SIGHUP reload cases and SIGUSR
equivalents use --require-handler. This effectively mitigates the case
we've seen time and time again where SIGHUP is used to rotate log files
or reload configs, but the sending site is mistakenly left present after
the removal of signal handler, resulting in unintended termination of
the process.

Signed-off-by: Chris Down <chris@chrisdown.name>
Gbp-Pq: Topic upstream
Gbp-Pq: Name 0007-kill-Support-mandating-the-presence-of-a-userspace-s.patch
misc-utils/kill.1.adoc
misc-utils/kill.c